Skip to content

GitOps drift: detection completeness + Argo CD API integration + Settings redesign#1138

Open
nadaverell wants to merge 27 commits into
mainfrom
feature/argo-api-diff
Open

GitOps drift: detection completeness + Argo CD API integration + Settings redesign#1138
nadaverell wants to merge 27 commits into
mainfrom
feature/argo-api-diff

Conversation

@nadaverell

@nadaverell nadaverell commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

A customer asked for "drift between deployments and GitOps." Radar already reads Argo's per-resource sync verdicts, but the detection had gaps, drift was hard to inspect, and the Settings surface (where an operator connects the integration this relies on) was a misleading single scroll. This PR closes all three: it completes credential-free drift detection, adds a hardened Argo CD API integration for connected clusters (the Git-rendered deep-diff, the commit behind a deployed revision, and repo-connection health), and reworks the Settings dialog into a sidebar with honest per-section apply semantics.

Everything is Argo-first (Flux/Git integration deliberately out of scope); cross-cluster/fleet rollups remain Radar Cloud's job.

What changed

1. Drift detection completeness (no credentials required)

  • Manual-sync drift reaches the issues stream. An Application on manual sync that stays OutOfSync continuously for >24h now emits a gitops_out_of_sync warning to /api/issues + MCP. Drift onset is tracked Radar-side per app UID (Argo records no sync-status transition time); fresh drift on a manual app stays a workflow state, not an alert.
  • New gitops_stale category — frozen sync verdicts. Argo only re-compares on refresh, so a down/wedged application-controller silently freezes every verdict. A down controller emits one critical rollup ("sync verdicts are frozen for N Applications"); a majority-stale fleet (≥3 apps) one warning rollup; isolated stale apps a per-app warning. Threshold max(30m, 10× argocd-cm timeout.reconciliation); controller pods located by label across all namespaces.
  • Cluster Audit: GitOps coverage. gitops-unmanaged-workload / gitops-helm-only-workload flag top-level workloads outside GitOps, classified via the existing pkg/subject ownership tiers. Gated on real GitOps root objects existing (not just CRDs); ownerRef'd workloads skipped; Argo label-tracking recognized.
  • ignoreDifferences coverage disclosure on the detail page: a plain-language note that some fields are excluded from drift comparison, with the rule count / target kinds / jq-and-managed-fields caveat behind an info tooltip.
  • Field diff descends into arrays. Drift entries now read as precise nested paths (e.g. spec.containers.[0].resources.limits.memory 128Mi → 256Mi) instead of dumping a whole array as one JSON blob — the common container/env/probe drift is finally legible.

2. Argo CD API integration (connected inspection)

Connecting the argocd-server API unlocks things that can't be derived from the Application CRD or the cluster alone — the canonical Git-rendered diff, the commit behind a deployed revision, and repo-connection health.

  • pkg/argoapi — a pure REST client (filtered managed-resources, revisions/{sha}/metadata, repositories, session/userinfo probe, version reachability, CLI-config token extraction) with no internal/ imports, so Radar Cloud can reuse it with its own credential store.
  • internal/argocd — service discovery by label, an argocd-owned port-forward for out-of-cluster runs, lazy self-reconnect from persisted config, context-switch reset, and per-call caches (15s managed-resources, bounded revision-metadata, non-blocking repo list).
  • Deep diff — GET .../resource-diff — the true Git-rendered desired vs normalized live YAML (absent from the last-applied annotation under SSA/Helm) + a structured field summary, reusing the shared drift walker.
  • Commit metadata — GET .../revision-metadata — turns a bare revision SHA in the status strip into the commit's author, subject, and signature status, so "what's running" becomes "what's running, by whom, and is it signed."
  • Repo health — the insights handler matches an app's source repositories against Argo's cached connection state to surface an otherwise-invisible top cause of stuck syncs. A failed repo connection is the cause of the ComparisonError Argo raises, so it folds into that one critical Issue (naming the repo + giving the fix action) rather than stacking a second alert beside it; it stands alone as a warning only when there is no ComparisonError (repo degraded but the app is still synced from cache). When Argo can't compare at all, the derivative per-resource Unknown sync renders muted with a single explanation while live health stays — one root cause, shown once.
  • Detail-page correctness — the resource a GitOps detail page draws is its graph's root, i.e. the page's subject; it no longer carries the portal treatment that made clicking it open a nested copy of the same page.
  • UI — Settings "Argo CD" card (URL / token / insecure-TLS / CLI-session adoption), an inline unified diff on the expanded change row with a full-screen maximize, and a "Connect Argo CD" hint when disconnected.

Security model — the load-bearing part reviewers should focus on. The Argo token is server-held transport; Radar's own per-user RBAC is the gate:

  • The diff endpoint enforces a dual gate before any upstream call: access to the Application, then a per-kind get SubjectAccessReview on the target resource. A user can never see manifests through Radar's token that their RBAC would deny.
  • Secret redaction is fail-closed: data/stringData/binaryData masked in any shape, annotation values masked, last-applied/managedFields/status stripped before serialization. No un-redact option.
  • The token never leaks: redacted from GET /api/config and from probe logs; upstream Argo error bodies (which can embed manifest/Secret content) are never reflected into responses.
  • The token is bound to its origin and cluster: changing the Argo URL requires re-entering it, and an auto-discovery token is bound to the kubeconfig context it was saved under — persisted, so the binding survives restarts (the in-cluster Service DNS is identical across clusters, so the URL alone can't distinguish them). The probe runs off an immutable snapshot (URL + token + TLS + frozen k8s client/config) and fails closed if a context switch races it — so the token can never reach a different cluster's argocd-server. Config is written 0600; the footer never persists an un-applied integration draft.
  • The metadata / repo-health enrichments gate on Application-namespace access (they surface app-scoped Git data, not cluster resources) and expose no manifests or Secrets: commit metadata renders a signature as an opaque chip. Repo health reports that a repo is unreachable plus Argo's connection error ("authentication required", "x509: …") for diagnosis — the same audience already sees the repo URL and its errors on the Application itself.

3. Settings dialog redesign

The dialog was one long scroll with a single Save labeled "applies on next launch" — but Prometheus and Argo apply live, so the label lied for them.

  • Two-pane sidebar with a status Overview landing and six sections (My permissions · Connection · Prometheus · Argo CD · AI diagnose · Advanced), widened dialog, sections kept mounted so a half-typed token survives navigation.
  • Overview at a glance: the landing shows what this Radar is connected to — cluster, Prometheus, Argo CD, MCP (with its copyable endpoint), and AI diagnose — each row a shortcut into its section. Backed by a read-only GET /integrations/argocd/status (configured/connected/address) alongside the existing Prometheus status, so the rows reflect live connection state.
  • Honest per-section semantics: startup config → "Takes effect on next launch" (footer Save, relabeled "Restart Radar to apply"); Prometheus/Argo → "Applies immediately" (own Connect/Apply, excluded from the footer); AI diagnose → self-saving.
  • Per-bucket normalized dirty drives sidebar dirty dots and the footer; a close guard confirms before discarding unsaved startup changes. The footer round-trips the last-committed integration values (never un-applied drafts), so a startup Save can't disturb a live integration.
  • Plain-language integration copy: each integration states what it enables — Prometheus (metrics graphs, usage history, rightsizing), Argo CD (the Git-rendered diff), MCP (AI-tool access) — matching the established framing; the Argo URL field is labeled "Server URL" rather than repeating the section title.
  • Constant dialog size + consistent tab shell: the dialog is a fixed height (short tabs breathe, tall ones scroll internally) instead of resizing on every tab switch; every tab shares one heading / label / button vocabulary, and the AI diagnose controls sit directly in the pane like the others rather than inside a nested card.
  • My permissions renders inline (identity, namespace selector, effective permissions, restricted resources) instead of a button that launched a separate modal. The AI diagnose tab is always present and, when no agent CLI is installed, explains the feature and how to enable it rather than disappearing.

Testing

  • Full Go suite (-race on the Argo manager) + tsc green in both modules (root + pkg). New tests cover the drift detectors, category/issues wiring, the audit coverage check, the array-descent diff, pkg/argoapi (deep-diff with fail-closed redaction + dual RBAC gate, revision metadata, repositories), the repo-health enrichment (failed / healthy / unknown / multi-source / project matching), and token handling — origin + context guard, _-namespace normalization, and the persisted context-binding round-trip.
  • Verified live against Argo CD (make gitops-demo: kind + Argo + real Git-backed apps):
    • Commit metadata — a deployed app's status strip resolves the commit's author + subject from the Argo API; the signature chip appears only for signed commits.
    • Repo health — inducing a failed repo connection folds into the app's ComparisonError as one repo-named critical Issue (no duplicate warning), and the resource list's derivative Unknown sync renders muted with a single explanation while health stays live.
    • Deep diff — Git-rendered desired-vs-live on an expanded change row.
    • Settings — the Argo connect flow, the Overview showing "Argo CD · Connected", honest per-section captions, the constant dialog size, and inline permissions (light + dark, no console errors).
  • scripts/gitops-demo.sh provisions a get-only Argo token; docs/gitops.md documents setup, the RBAC/redaction guarantees, and the degradation story.

Notes

  • Argo CD core installs (no API server) and hub-spoke topologies degrade gracefully to the annotation-based drift view — documented.
  • Repo health needs the Argo token to carry repositories, get (the deep diff only needs applications, get); the connection-error text is shown for diagnosis, matching what the Application already surfaces.
  • The Settings dialog has no automated test today (consistent with the pre-existing dialog); it's covered by tsc + the visual pass. The one behavior worth a future guard is "the footer round-trips committed integration values, never drafts."

Note

High Risk
Changes touch server-held Argo tokens, cross-cluster binding, dual RBAC on manifest fetches, and structural Secret redaction—security-critical paths with substantial new surface area.

Overview
Argo CD API integration wires Settings/PUT /api/integrations/argocd (probe-before-persist, token preserved on GET redaction) to a new internal/argocd manager with in-cluster discovery, optional port-forward, context-bound auto-discovery tokens, and caches for managed resources, revision metadata, and repositories. New routes serve Git-rendered desired vs live (resource-diff, dual RBAC + fail-closed Secret redaction) and revision commit metadata, and GitOps insights advertise capabilities only when managed-resources actually works; repo connection failures enrich or fold into ComparisonError issues.

Credential-free drift gains manual-sync OutOfSync warnings after a 24h gate (tracked per Application UID), stale comparison rollups when the application-controller is down or most apps are stale, new gitops_stale / OutOfSyncManual issue categories, and audit input that treats GitOps as present only when real Argo/Flux root objects exist (with Argo app names for label-tracking coverage).

Config/security: Argo URL/token/TLS/context fields in config.json, file writes at 0600, token redaction on config APIs, integration fields immune to generic PUT /api/config, and argocd.Reset() on kube context switch. UI: optional renderRevisionMeta on the GitOps detail status strip for inline commit info when connected.

Reviewed by Cursor Bugbot for commit 951918a. Bugbot is set up for automated code reviews on this repo. Configure here.

@nadaverell nadaverell requested a review from hisco as a code owner July 9, 2026 08:40
Comment thread internal/server/argo_diff.go
Comment thread internal/server/gitops_handlers.go
Comment thread internal/server/argo_diff.go
Comment thread internal/argocd/manager.go
Two coordinated capabilities for the "drift between deployments and GitOps"
ask, credential-free detection plus a hardened connected deep-diff view.

## Detection (no credentials)

- Manual-sync Applications continuously OutOfSync for >24h reach the issues
  stream (gitops_out_of_sync, warning). Drift onset is tracked Radar-side per
  app UID on the ResourceCache (Argo records no sync-status transition time);
  a restart resets the clock — deliberately conservative.
- New gitops_stale category: a down application-controller emits one critical
  rollup ("sync verdicts are frozen"), a majority-stale fleet (>=3 apps) one
  warning rollup, isolated stale apps per-app warnings. Threshold max(30m,
  10x argocd-cm timeout.reconciliation); controller pods found by label across
  all namespaces.
- Cluster Audit: gitops-unmanaged-workload / gitops-helm-only-workload flag
  top-level workloads outside GitOps via the pkg/subject overlay tiers. Gated
  on real GitOps root objects existing (not just CRDs); ownerRef'd workloads
  skipped; Argo label-tracking recognized by cross-referencing app names.
- GitOps detail page discloses spec.ignoreDifferences as comparison coverage
  (rule/jq/managed-fields counts + kinds).

## Argo CD API deep diff (connected)

- pkg/argoapi: pure REST client for argocd-server (filtered managed-resources,
  session/userinfo probe, version reachability, CLI-config token extraction);
  no internal/ imports, so radar-hub can reuse it.
- internal/argocd: discovery by service label, argocd-owned port-forward for
  out-of-cluster runs, 15s TTL cache, lazy self-reconnect, context-switch reset.
- GET /api/argo/applications/{ns}/{name}/resource-diff: dual RBAC gate
  (Application namespace access + per-kind get SAR) before any upstream call;
  Git-rendered desired vs normalized live YAML + field entries.
- UI: Settings "Argo CD" card (URL/token/insecure-TLS/CLI-session), inline
  unified diff on the expanded change row with a full-screen maximize, and a
  "Connect Argo CD" hint when disconnected.

## Security model (3 review rounds, all findings fixed)

- Token bound to its origin AND kubeconfig context: changing the URL or
  switching context requires re-entering it; the probe runs off an immutable
  snapshot (URL + token + TLS + frozen k8s client/config), fails closed if a
  switch races the capture, and commits only under a generation check — the
  token never reaches a different cluster's argocd-server.
- Secret redaction is fail-closed: data/stringData/binaryData masked in any
  shape, annotation values masked, last-applied/managedFields/status stripped
  before serialization. No un-redact.
- No credential or secret in responses or logs: token redacted from GET
  /api/config and probe logs; upstream Argo error bodies never reach responses.

Verified: full Go suite + tsc green both modules; real-cluster validation
against nonprod GKE Argo (discovery/port-forward/token-verify round-trip);
diff render + redaction + UI verified on the gitops-demo kind cluster.
Comment thread internal/server/argocd_integration.go Outdated
if p.TargetPort.IntVal > 0 {
return int(p.TargetPort.IntVal)
}
return int(p.Port)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Named Service targetPort mis-forwarded

Low Severity

Out-of-cluster discovery port-forwards using resolveTargetPort, which falls back to the Service port when TargetPort is a name (not an integer). Forwarding the Service port onto the pod often fails, so auto-discovery can incorrectly report Argo CD unreachable.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8096d83. Configure here.

Comment thread pkg/argoapi/client.go Dismissed
Comment thread internal/server/argo_diff.go Fixed
Comment thread internal/server/argo_diff.go Fixed
Comment thread internal/server/argo_diff.go Fixed
Comment thread internal/server/argocd_integration.go Fixed
Comment thread internal/server/argocd_integration.go Fixed
The field diff compared arrays by serialized equality, so a single field
changing inside one list element (the common case: a container's image,
resources, or env) rendered the whole array as an unreadable one-line JSON
blob on both sides. diffValues now descends element-by-element, so that
change reads as its own path — e.g. spec.containers.[0].resources.limits.memory
changed 128Mi → 256Mi. Index alignment is accurate for these inputs: K8s
controllers preserve list order between apply and observe, and the Argo API
normalizes both sides. A length change surfaces the added/removed element at
its index. Applies to both the last-applied and Argo-API (DiffObjects) paths.
The note led with Argo-internal jargon (rule counts, "jq expressions,
managed-fields managers") on everyone's screen. Now it leads with a plain
one-liner — "some fields are intentionally excluded from drift comparison, so
a resource can read as in sync even if those fields differ" — and moves the
expert detail (rule count, target kinds, the jq/managed-fields caveat) behind
a "details" info tooltip. Also shrinks the note's footprint in the Resources
header.
Comment thread internal/audit/runner.go
argoAppNames = make(map[string]struct{}, len(apps))
for _, app := range apps {
argoAppNames[app.GetName()] = struct{}{}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argo app names ignore namespace

Low Severity

gitOpsRoots builds ArgoAppNames keyed only by Application name, not namespace. Two Applications with the same name in different namespaces collapse to one map entry, so GitOps coverage can treat unrelated app.kubernetes.io/instance labels as managed by Argo incorrectly.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 030848e. Configure here.

The Sync/Health badges and the action button were pinned to the top of
each row (self-start), so in a tall row (multi-line name + drift/message)
they floated above the row's midline. Center them (self-center) against
the row height.
Replaces the single-scroll settings dialog with a two-pane layout (left
sidebar nav + right content) and makes each section's save/apply semantics
truthful instead of one global "applies on next launch" footer that lied for
the live integrations.

- 6 flat sidebar sections: My permissions · Connection (kubeconfig/namespace/
  port/browser) · Prometheus · Argo CD · AI diagnose · Advanced (MCP/timeline/
  history). Dialog widened to max-w-4xl; sections kept mounted (hidden, not
  unmounted) so a half-typed Argo token survives nav.
- Honest per-section captions: startup config → "Takes effect on next launch";
  Prometheus/Argo → "Applies immediately — no restart"; AI → "new investigations".
- The footer now scopes ONLY to restart-required startup config and is labeled
  "Restart Radar to apply". Live integrations own their Apply/Connect and never
  ride the footer; AI is self-saving (personal, client-side).
- Per-bucket normalized dirty (replaces the coarse boolean) drives sidebar dirty
  dots and the footer; a close guard confirms before discarding unsaved startup
  changes.

Correctness: PUT /api/config is full-replacement, so the footer still sends the
full config — but round-trips the last-COMMITTED integration values (kept fresh
via onApplied), never un-applied input drafts, so a typed-but-not-connected Argo
URL can't ride a startup Save and silently clear the stored token.
Comment thread internal/server/argo_diff.go
…modal

The "My permissions" section was a near-empty pane whose only content was an
"Open my permissions" button that launched a separate full-screen dialog — a
pointless hop, and it was the default landing section. Extracted the
permissions viewer into MyPermissionsContent (no modal chrome, apiserver
queries gated on the section being active) and render it directly in the
settings pane. Opening Settings now lands on your actual access inline.
Removed the standalone MyPermissionsDialog usage + onShowMyPermissions
launcher plumbing from App and the dialog props (Settings was its only opener).
pods.md, scratchpad/, AUDIT-*.md and other local session notes were
accidentally swept into an earlier commit; they aren't part of this change.
@nadaverell nadaverell changed the title Argo CD API deep diff — Git-rendered desired-vs-live with hardened credential handling GitOps drift: detection completeness + Argo CD API deep diff (+ Settings dialog redesign) Jul 9, 2026
The repo forbids native title attributes (check-no-native-title) in favor
of the themed Tooltip. Wrap the disabled-tab hint and the unsaved-changes
dot in Tooltip.
Comment thread internal/k8s/detect_gitops.go
…permissions namespace

- The startup-config PUT no longer owns any live integration field. It
  preserves prometheusUrl/headers and argoCdUrl/token/insecureTls verbatim
  (the /integrations/* endpoints are their sole owners), so a full-config
  save can't race an in-flight Apply/Connect and revert — or clear the token
  on — a live integration.
- ignoreDifferences JSON pointers into array indices now match the bracketed
  drift paths that array descent emits: /spec/containers/0/image strips
  spec.containers.[0].image. Without this, exclusions into arrays silently
  stopped working after per-index diffing landed.
- My permissions lands on an accessible namespace: if the default isn't in the
  user's namespace list and they haven't picked one, it switches to the first
  accessible one instead of showing an empty "default".
Comment thread internal/argocd/manager.go
- Serialize probes (probeMu) so background reconnect and the on-demand
  probe from ManagedResourcesCached can't race on the port-forward/client.
- Fail closed on a context/token mismatch in auto-discovery: Probe refuses
  (errTokenContextMismatch) instead of connecting unauthenticated, which
  would light deep-diff while every managed-resource call 403s.
- Only rebind tokenContext for an explicitly-provided token (tokenIsFresh);
  re-saving with a carried-forward token no longer rebinds it to the
  current cluster context.
- Offer deep-diff via IsConfigured() right after restart while the
  background reconnect is still in flight, instead of 503 until first probe.
- Reject an empty Application namespace on the resource-diff endpoint (400)
  rather than skipping the namespace RBAC gate.
- Rewrite sanitizeForLog with strings.ReplaceAll (a recognized log-injection
  barrier) and sanitize wrapped error strings at the argo log sites.
Comment thread internal/server/argo_diff.go
Settings now lands on an Overview that shows what this Radar is connected to
at a glance — cluster, Prometheus, Argo CD, MCP (with its endpoint), and AI
diagnose — each row a shortcut into its section. Backed by a new read-only
GET /integrations/argocd/status (configured/connected/address, mirroring the
Prometheus status endpoint), so the Argo row reflects live connection state.

Also from the settings product review:
- Nav is ordered as a narrative (Overview, My permissions, Connection,
  Prometheus, Argo CD, AI diagnose, Advanced) and AI diagnose is always shown.
- When no agent CLI is installed, the AI tab explains the feature and how to
  enable it instead of vanishing.
- Prometheus and MCP now state what they enable (metrics/graphs; AI-tool
  access), matching Argo's framing; the Argo URL field is labeled 'Server URL'
  instead of repeating the section title.
The dialog height was content-driven, so it grew and shrank on every tab
switch — distracting and made short tabs feel cramped. Pin it to a fixed
height (capped to the viewport on short screens): tall tabs scroll inside the
content pane, short tabs get calm whitespace. Reserve the scrollbar gutter so
switching between a scrolling tab and a short one doesn't shift content
sideways.
- AI diagnose now uses the same heading block as every other tab (h3 +
  description) instead of a nested bordered card with its own smaller heading;
  the agent controls sit directly in the pane.
- Argo CD: label the token field, explain how to get one (argocd account
  generate-token / the UI), frame the CLI session as the alternative, and add
  a Server URL hint — the tab was cryptic for anyone who doesn't already know
  Argo's auth model.
- Prometheus 'Apply now' now uses the primary button style, matching Argo's
  'Connect & save' — the two live integrations share one button vocabulary.
The status strip showed a bare revision SHA. When the Argo CD integration is
connected, hydrate it with the commit's author, subject, and signature status
pulled from the Argo CD API server — so "what's running" becomes "what's
running, by whom, and is it signed."

- pkg/argoapi: RevisionMetadata client method (GET .../revisions/{rev}/metadata),
  passing appNamespace/project/sourceIndex for multi-source apps. Fields are
  best-effort (they vary across Argo versions); signature is rendered as an
  opaque signed/unverified chip, not parsed.
- internal/argocd: RevisionMetadataCached with a bounded TTL/LRU cache, cleared
  on reconnect/context switch so it never serves another cluster's data.
- GET /argo/applications/{ns}/{name}/revision-metadata — gated on Application-
  namespace access only (app-scoped Git data, not a K8s resource) and on the
  integration being connected. New capabilities.revisionMetadataAvailable flag.
- Frontend: useArgoRevisionMetadata hook + RevisionMetaChip, wired into the
  status strip via an additive renderRevisionMeta slot on the shared layout.
  Renders nothing until data arrives, so the SHA is never blocked.
A failed Git repo connection is a top cause of stuck syncs and is invisible
from the Application CRD. When the Argo CD integration is connected, the
insights handler now matches the app's source repositories against Argo's
cached connection state and surfaces a warning Issue when one is unreachable —
rendered in the existing Issues band, no new UI.

- pkg/argoapi: Repositories client method (GET /api/v1/repositories), with the
  repo's project so same-URL repos in different projects aren't conflated.
- internal/argocd: RepositoriesCached is non-blocking — it returns a cached (or
  nil) value immediately and refreshes in the background, so repo health never
  adds the Argo client timeout to the 2s insights poll. Cache cleared on
  reconnect/context switch; a generation guard prevents caching another
  cluster's repos.
- Enrichment runs in the server handler (pkg/gitops/insights stays pure and
  credential-free), best-effort: any Argo error is skipped, an unmatched repo is
  treated as unknown (never asserted healthy), and the raw Argo error is not
  surfaced. Project-aware, multi-source + source-hydrator aware, with URL
  normalization that matches ssh/https forms of the same repo.
Comment thread internal/server/argocd_integration.go
Include the raw Argo connection message (e.g. "authentication required",
"x509: certificate signed by unknown authority") in the issue's rawMessage so
the operator can see why the repo is unreachable — the same audience already
sees the repo URL and repo errors on the Application itself.
Comment thread internal/server/argo_diff.go
Reconciled the 3 conflicts:
- config.go: kept both new field sets (Argo CD URL/token/TLS + AI history/consent).
- AISettings.tsx: kept main's clear-history action (ClearHistoryRow +
  onHistoryCleared) inside this branch's card-less AI-diagnose tab layout.
- SettingsDialog.tsx: kept this branch's tabbed redesign; re-applied main's
  onHistoryCleared wiring and its autocorrect-off <Input> for the text fields.

Verified on the merged tree: go build + go test (pkg + internal), tsc
(web + k8s-ui), frontend build, native-title guard — all green.
Triage of the Bugbot review on the merged PR:
- errTokenContextMismatch now wraps ErrTokenInvalid, so a token bound to a
  different cluster context surfaces as a 're-authenticate' prompt (403 /
  'token rejected') everywhere instead of a misleading 502 'bad gateway' or
  'unreachable'.
- The Argo resource-diff and revision-metadata handlers normalize the '_'
  empty-namespace placeholder the web client sends, so it isn't treated as a
  literal namespace by RBAC or the upstream appNamespace lookup.
Comment thread internal/argocd/manager.go
An auto-discovery (empty-URL) token is bound to the kubeconfig context it was
configured under, but that binding (tokenContext) was only in memory — never
persisted. After a restart, seed left it empty, so Probe treated the token as a
context mismatch every time and it could never reconnect (while IsConfigured
still advertised deep-diff). Persist it as ArgoCDTokenContext and restore it on
seed; a same-context restart now reconnects, while a genuinely different context
still refuses (restoring it to the CURRENT context would have defeated that
cross-cluster guard). Preserved through the full-replacement PUT /config.
@nadaverell nadaverell changed the title GitOps drift: detection completeness + Argo CD API deep diff (+ Settings dialog redesign) GitOps drift: detection completeness + Argo CD API integration + Settings redesign Jul 10, 2026
- Rename the footer's 'Reset' to 'Discard changes' and make it revert edits to
  the last-saved values instead of clearing every startup field to defaults —
  the button now does what it looks like it does, non-destructively.
- Drop the pre-save 'Restart Radar to apply' footer text (the section caption
  and the post-save toast already say it) and slide the footer in/out instead
  of hard-appearing.
- Explain 'Open browser on start' with a description line, and give the Browser
  field OS-aware examples (macOS app name vs. Linux/Windows command). Adds an
  optional description slot to ConfigToggle.
The Argo tab had a 'Use Argo CD CLI session' button the user had to know about
and click blind — it errored if no session existed. Now Radar reads the CLI
config (~/.config/argocd/config) and, when a current-context login is present,
surfaces it as a one-click card ('Argo CD CLI session found — <user> @ <server>
· Use this session') with manual token entry demoted to 'Or paste a token.'
When no session exists, neither the card nor the button appears.

Backend exposes a token-free { server, user, insecure } on GET /api/config
(the token stays server-side, materialized only on connect). 'Use this session'
connects to the session's own server + TLS mode, so it works even when the URL
field points elsewhere.
Comment thread internal/argocd/manager.go
An 'argocd login' done over a port-forward records the server as 127.0.0.1:<port>,
which is only reachable while that forward runs — almost never when Radar reads the
config later. Offering it as a one-click just failed 'unreachable' on click, so
suppress loopback sessions from detection; the user connects via the URL field for
those. Real hostnames / in-cluster services (the stable case) are still offered.
Comment thread internal/server/argocd_integration.go Outdated
The tree the detail page draws has the viewed resource as its root, and that
root was tagged as a GitOps portal node (tool badge, chevron, click-to-dive
ring). Clicking the resource you're already looking at navigated to a fresh
copy of the same page, stacking an identical breadcrumb segment each time.
The root is the subject, not a portal: drop its portal treatment, and no-op a
self-referential open so a page can never nest inside itself.
When Argo can't reach the source repo it can't build the desired state, so it
raises a ComparisonError and every managed resource's sync goes Unknown. That
one root cause was surfacing as three things: a critical ComparisonError, a
separate RepoUnreachable warning, and a wall of Unknown sync pills across every
resource row — one problem rendered as many.

Point the page at what matters. A failed repo connection now folds into the
existing ComparisonError (naming the repo + giving the fix action, keeping its
critical severity) instead of stacking a second alert; RepoUnreachable stays
standalone only when there's no ComparisonError. And when the app can't compare
at all, the derivative per-row Unknown sync renders muted with a single
explanation, while live health stays.
…+ free the port-forward on auth failure

Two defects in the Argo credential model surfaced by review:

- A failed connect attempt with a fresh token stamped tokenContext to the current
  cluster before the probe, and the rollback re-pointed the manager with a plain
  SetConfig that leaves tokenContext untouched — so the restored (older)
  auto-discovery token was left marked valid for the wrong cluster in memory,
  defeating the cross-cluster guard until restart. Rollback now restores the
  captured prior binding via RestoreConfig on both failure paths (probe + persist).

- Out-of-cluster discovery installs a port-forward to a reachable argocd-server
  before verifying the token; a token failure returned without tearing it down,
  leaving a live localhost tunnel while the manager reports disconnected. Probe
  now drops the forward on auth failure (unless a config change already owns
  cleanup).
// only; the client gets a generic message.
log.Printf("[argo] resource-diff for %s/%s failed: %s", sanitizeForLog(namespace), sanitizeForLog(name), sanitizeForLog(err.Error()))
s.writeError(w, http.StatusBadGateway, "Failed to fetch the diff from the Argo CD API server.")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing app returns bad gateway

Low Severity

writeArgoDiffError never maps argoapi.ErrNotFound, so a missing Application (or other upstream 404) becomes a 502 Bad Gateway. The sibling revision-metadata handler already maps that sentinel to 404.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 562629c. Configure here.

…ne on expand

The per-resource Argo diff was offered whenever the integration was merely
configured, behind a 'Full diff' button revealed only after expanding a row —
so a resource expanded to show a lone button that then dead-ended on a rejected
or under-scoped token, one integration problem rendered as N failing buttons.

- Gate the diff (and revision-metadata) capability on the app's managed-resources
  call — the diff's own data source — not IsConfigured() or the userinfo probe.
  A token that authenticates but 403s on managed-resources no longer advertises
  diffs; the app surfaces one 'check Argo CD in Settings' hint instead. The probe
  is 15s-cached and shared with the diff endpoint, so per-row diffs reuse it.
- Surface ArgoConfigured separately so the hint distinguishes 'not set up'
  (Connect) from 'set up but not serving' (check Settings).
- Render the diff inline on expand rather than behind a second click, with a
  subtle skeleton in place of the full-pane radar loader — the capability gate
  means it only mounts when the diff can be served.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 4 total unresolved issues (including 3 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 951918a. Configure here.

items, err := client.ManagedResources(ctx, q)
if err != nil {
return nil, err
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead Argo connection never recovers

Medium Severity

Once Get has a non-nil client, transport failures never invalidate it or re-run Probe. After an out-of-cluster port-forward dies (argocd-server restart, node drain), ManagedResourcesCached and RevisionMetadataCached keep calling the dead localhost client and return errors indefinitely. Deep diff, commit metadata, and ArgoDiffAvailable stay broken until Settings re-save or a Radar restart. Prometheus’s EnsureConnected re-probes and rediscovers on failure; this manager does not.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 951918a. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants